Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated %d to %@ when formatting the bookmark ID #3

Merged
merged 3 commits into from Sep 12, 2011

Conversation

mauricerkelly
Copy link
Contributor

I found that when using %d the wrong value was being output. I noted from documentation that on a 64-bit build the NSInteger would be 64 bits in size, whereas for a 32-bit build NSInteger is just 32 bits in size.

Using %@ to "print" the NSInteger seemed to work fine - it actually gave me a representation that matched the values on instapaper.com. Can you try this out and see if it works for you?

I also fixed the calls to bookmark.hash as I suspect that should have been bookmark.hashString.

Cheers,

Maurice

@matthiasplappert
Copy link
Owner

You are absolutely right about the .hashString. Thanks for fixing that.

Regarding the NSInteger issue: using %d is in fact not working if you compile for a 64bit environment. However, %@ is not the proper way to handle this issue. Apple recommends that you use %ld instead and cast the var to long. Example:

NSInteger i = 42;
NSString *string = [NSString stringWithFormat:@"%ld", (long)i];

Can you change that so that I can merge your pull request?

@mauricerkelly
Copy link
Contributor Author

I'll have to do some more testing, because switching to %ld does not have any improvement for me.

As a bit of background, I'm pulling down the bookmarks and storing them in Core Data - the bookmark ID is being stored in an "Integer 64" attribute type. The format is only coming out badly when it's been through Core Data so it might be getting mangled slightly as I move it in and out.

I'll do some more on it this evening if that's okay.

Cheers,

Maurice

@mauricerkelly
Copy link
Contributor Author

I think part of the problem is on my end with putting the NSInteger into and out of a Core Data store. I've updated my commits as you suggested and I'll figure out my own problem at a later stage :-)

Cheers,

Maurice

@mauricerkelly
Copy link
Contributor Author

@matthiasplappert - is everything okay with the updated pull request

matthiasplappert added a commit that referenced this pull request Sep 12, 2011
Update %d to %ld when formatting the bookmark ID. Update to latest JSONKit commit.
@matthiasplappert matthiasplappert merged commit b186902 into matthiasplappert:master Sep 12, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants